wxGUI/psmap: fix close vector map properties dialog after hit OK button #3085

tmszi · 2023-07-14T11:10:37Z

cmbarton · 2023-09-12T19:25:03Z

Has this been merged yet? Doesn't look like it.

petrasovaa · 2023-09-13T03:04:27Z

Has this been merged yet? Doesn't look like it.

Not yet. I looked at it briefly some time ago and I had some issues reproducing it. Can you confirm this PR actually fixes it?

tmszi · 2023-09-13T19:20:47Z

Has this been merged yet? Doesn't look like it.

Not yet. I looked at it briefly some time ago and I had some issues reproducing it. Can you confirm this PR actually fixes it?

The bug is still reproducible following the steps in the reported issue #3084 and this PR fixes it. I carefully tested each of my PRs before creating them.

cmbarton · 2023-09-13T21:07:48Z

Unfortunately, this does not fix the bug. I compiled this PR and it is still there. It behaves as if closing the vector settings dialog does not relinquish the focus, even though the window is closed.

No other windows (cartographic composer layer window, main cartographic composer window, main GUI window) are responsive as if another dialog was waiting for a response.

petrasovaa · 2023-09-14T03:43:22Z

Sorry it took so long, but finally I got to this. @tmszi yes, I can reproduce it on Ubuntu and I agree with the fix. The problem is ShowModal is not called the standard way with this dialog (testing outcome of ShowModal and then destroying the dialog would be better, but would require larger changes).

@cmbarton since this seems to fix it for Ubuntu (and I assume @tmszi tested it on linux as well, no?), could you make sure the patch is correctly applied? But it sounds like maybe the behavior with this fix is different? It could be something Mac specific.
Maybe trying self.EndModal(wx.ID_OK) instead of self.Destroy() may make a difference?

tmszi · 2023-09-14T11:21:00Z

Sorry it took so long, but finally I got to this. @tmszi yes, I can reproduce it on Ubuntu and I agree with the fix. The problem is ShowModal is not called the standard way with this dialog (testing outcome of ShowModal and then destroying the dialog would be better, but would require larger changes).

My complex debug info:

Problematic is line vector map props dialog only, especially PenStyleComboBox() widget, respectively base class OwnerDrawnComboBox widget.

grass/gui/wxpython/psmap/dialogs.py

Lines 2927 to 2929 in 69dd324

    
           self.styleCombo = PenStyleComboBox( 
        
               panel, choices=penStyles, validator=TCValidator(flag="ZERO_AND_ONE_ONLY") 
        
           )

If you try comment code lines 2927-2960, confirming line vector map props dialog Ok button widget works as expected.

Others vector map props dialog (point and area type), confirming Ok button widget works as expected. Because doesn't use PenStyleComboBox widget.

Another bug (new PR) is in PenStyleComboBox class OnDrawItem method (DrawText and DrawLine method), under wxGTK.

Traceback (most recent call last):
  File "/usr/lib64/grass84/gui/wxpython/psmap/dialogs.py", line 182, in OnDrawItem
    dc.DrawText(
TypeError: DC.DrawText(): arguments did not match any overloaded call:
  overload 1: argument 3 has unexpected type 'float'
  overload 2: argument 2 has unexpected type 'int'

grass/gui/wxpython/psmap/dialogs.py

Lines 182 to 192 in 69dd324

    
           dc.DrawText( 
        
               self.GetString(item), 
        
               r.x + 3, 
        
               (r.y + 0) + ((r.height / 2) - dc.GetCharHeight()) / 2, 
        
           ) 
        
           dc.DrawLine( 
        
               r.x + 5, 
        
               r.y + ((r.height / 4) * 3) + 1, 
        
               r.x + r.width - 5, 
        
               r.y + ((r.height / 4) * 3) + 1, 
        
           )

@cmbarton since this seems to fix it for Ubuntu (and I assume @tmszi tested it on linux as well, no?),

I tested this PR under wxGTK and wxMSW.

nilason · 2023-09-14T11:54:19Z

I can confirm that the problem persists on Mac even with this present solution, using
Python 3.11.5 (main, Aug 25 2023, 13:55:04) [Clang 13.1.6 (clang-1316.0.21.2.5)]
wxPython 4.2.1 osx-cocoa (phoenix) wxWidgets 3.2.2.1

nilason · 2023-09-14T12:47:19Z

The vector property window, once opened cannot be closed. By simply open it, doing nothing, then close (via cancel button), only puts the window behind all other windows, but the window doesn't close nor the modal state cannot be terminated. Only solution is to force quit gui (wxPython).

nilason · 2023-09-14T12:47:57Z

I tried commenting out the lines 2927-2960, but it didn't do any difference.

petrasovaa · 2023-09-14T13:17:06Z

Maybe trying self.EndModal(wx.ID_OK) instead of self.Destroy() may make a difference?

Could someone on Mac try using this in the OnOK method?

nilason · 2023-09-14T13:30:35Z

Maybe trying self.EndModal(wx.ID_OK) instead of self.Destroy() may make a difference?

Could someone on Mac try using this in the OnOK method?

I missed that suggestion, yes it seems to work! Now, only cancel remain the same (problematic).

nilason · 2023-09-14T14:03:01Z

Following works for me:

    def OnCancel(self, event):
        self.EndModal(wx.ID_CANCEL)

    def OnOK(self, event):
        self.update()
        event.Skip()
        self.EndModal(wx.ID_OK)

One (rather trivial, but still) problem remains, after closing "Vector Properties" window, the "Map settings" window looses focus and is placed behind "Cartographic Composer" window.

cmbarton · 2023-09-14T17:41:12Z

One (rather trivial, but still) problem remains, after closing "Vector Properties" window, the "Map settings" window looses focus and is placed behind "Cartographic Composer" window.

This problem seems to be pervasive in the custom-designed interfaces that wrap GRASS commands (e.g., the raster and vector import tools). Trivial but annoying.

In class today, I needed to show a student why they could not import a file. After selecting the file in the raster import tool, the import window disappears behind the main GRASS GUI window. They did not know this and wondered what happened.

nilason · 2023-09-14T18:38:14Z

One (rather trivial, but still) problem remains, after closing "Vector Properties" window, the "Map settings" window looses focus and is placed behind "Cartographic Composer" window.

This problem seems to be pervasive in the custom-designed interfaces that wrap GRASS commands (e.g., the raster and vector import tools). Trivial but annoying.

In class today, I needed to show a student why they could not import a file. After selecting the file in the raster import tool, the import window disappears behind the main GRASS GUI window. They did not know this and wondered what happened.

Yes, now with the single window mode it is particularly annoying because it is virtually impossible to see (if the main window fills the screen). I was fooled by this several times testing this PR.

…m PsmapDialog

petrasovaa · 2023-09-15T03:56:16Z

Following works for me:

    def OnCancel(self, event):
        self.EndModal(wx.ID_CANCEL)

    def OnOK(self, event):
        self.update()
        event.Skip()
        self.EndModal(wx.ID_OK)

I decided to slightly refactor this, the dialog is now derived from standard wx dialog, so the hack above is not needed. Testing welcome. I did have some issues with a validator, so I removed it, it's not very important anyway,

petrasovaa · 2023-09-15T04:01:48Z

One (rather trivial, but still) problem remains, after closing "Vector Properties" window, the "Map settings" window looses focus and is placed behind "Cartographic Composer" window.

That looks Mac-specific, you could try calling self.Raise() after vector properties dialog is closed.

But some of these problems with single window are general, we could explore some window settings like 'wx.FRAME_FLOAT_ON_PARENT'.

tmszi · 2023-09-15T05:38:09Z

Successfully tested on wxGTK, wxMSW.

nilason · 2023-09-15T08:02:12Z

I decided to slightly refactor this, the dialog is now derived from standard wx dialog, so the hack above is not needed. Testing welcome. I did have some issues with a validator, so I removed it, it's not very important anyway,

That works fine!

nilason · 2023-09-15T08:07:22Z

One (rather trivial, but still) problem remains, after closing "Vector Properties" window, the "Map settings" window looses focus and is placed behind "Cartographic Composer" window.

That looks Mac-specific, you could try calling self.Raise() after vector properties dialog is closed.

But some of these problems with single window are general, we could explore some window settings like 'wx.FRAME_FLOAT_ON_PARENT'.

I found what I believe a better solution. In VectorPanel::OnProperties() initialise the VPropertiesDialog with self.parent instead of the present self. "self" is only the Panel, but we need to have the MapDialog as parent.

gui/wxpython/psmap/dialogs.py

cmbarton · 2023-09-15T16:41:53Z

If this is committed I can test too.

petrasovaa · 2023-09-15T17:14:27Z

I found what I believe a better solution. In VectorPanel::OnProperties() initialise the VPropertiesDialog with self.parent instead of the present self. "self" is only the Panel, but we need to have the MapDialog as parent.

That makes sense. I assume you tested it, does this solve the problem with hiding the dialog on Mac?

nilason · 2023-09-15T17:24:06Z

That makes sense. I assume you tested it, does this solve the problem with hiding the dialog on Mac?

Yes

cmbarton · 2023-09-15T19:18:36Z

This solves all reported problem with the cartographic composer. Thanks to all!

Can we add the fix to the hidden window to the raster and vector import tools too? Probably some other places need it, but I don't know which right now.

nilason · 2023-09-15T19:23:18Z

This solves all reported problem with the cartographic composer. Thanks to all!

👍

Can we add the fix to the hidden window to the raster and vector import tools too? Probably some other places need it, but I don't know which right now.

I looked for similar problems in the import tools, but I couldn't find any. Please, take a closer look and report in an another ticket.

cmbarton · 2023-09-15T19:24:47Z

OK. Thanks again to you, Thomas, and Anna for fixing this. Michael

…

____________________ Michael Barton

On Sep 15, 2023, at 12:23 PM, Nicklas Larsson ***@***.***> wrote: This solves all reported problem with the cartographic composer. Thanks to all! 👍 Can we add the fix to the hidden window to the raster and vector import tools too? Probably some other places need it, but I don't know which right now. I looked for similar problems in the import tools, but I couldn't find any. Please, take a closer look and report in an another ticket. — Reply to this email directly, view it on GitHub <#3085 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACENLLYZRVHUKMYEOIMSOTLX2STLBANCNFSM6AAAAAA2KF56ZA>. You are receiving this because you were mentioned.

…on (OSGeo#3085) * make VPropertiesDialog behave like standard dialog, don't inherit from PsmapDialog * accept suggestion about parent to prevent hiding the dialog on wxMAC --------- Co-authored-by: Anna Petrasova <[email protected]>

tmszi added bug Something isn't working GUI wxGUI related backport to 8.3 backport to 7.8 PR needs to be backported to release branch 7.8 labels Jul 14, 2023

tmszi added this to the 8.3.1 milestone Jul 14, 2023

wxGUI/psmap: fix close vector map properties dialog after hit OK button

9763ed3

tmszi force-pushed the wxgui-psmap-fix-click-vprops-dialog-ok-btn branch from c6d2f44 to 9763ed3 Compare July 14, 2023 19:35

tmszi removed the backport to 7.8 PR needs to be backported to release branch 7.8 label Jul 15, 2023

tmszi requested a review from petrasovaa July 15, 2023 07:25

tmszi modified the milestones: 8.3.1, 8.4.0 Jul 18, 2023

make VPropertiesDialog behave like standard dialog, don't inherit fro…

744d2cb

…m PsmapDialog

nilason reviewed Sep 15, 2023

View reviewed changes

gui/wxpython/psmap/dialogs.py Outdated Show resolved Hide resolved

accept suggestion about parent

96fc1c6

tmszi merged commit fd59e94 into OSGeo:main Sep 16, 2023
18 checks passed

tmszi deleted the wxgui-psmap-fix-click-vprops-dialog-ok-btn branch September 16, 2023 04:22

tmszi removed backport to 8.3 labels Sep 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wxGUI/psmap: fix close vector map properties dialog after hit OK button #3085

wxGUI/psmap: fix close vector map properties dialog after hit OK button #3085

tmszi commented Jul 14, 2023

cmbarton commented Sep 12, 2023

petrasovaa commented Sep 13, 2023

tmszi commented Sep 13, 2023

cmbarton commented Sep 13, 2023

petrasovaa commented Sep 14, 2023 •

edited

Loading

tmszi commented Sep 14, 2023 •

edited

Loading

nilason commented Sep 14, 2023

nilason commented Sep 14, 2023

nilason commented Sep 14, 2023

petrasovaa commented Sep 14, 2023

nilason commented Sep 14, 2023

nilason commented Sep 14, 2023

cmbarton commented Sep 14, 2023

nilason commented Sep 14, 2023

petrasovaa commented Sep 15, 2023

petrasovaa commented Sep 15, 2023

tmszi commented Sep 15, 2023

nilason commented Sep 15, 2023

nilason commented Sep 15, 2023

cmbarton commented Sep 15, 2023

petrasovaa commented Sep 15, 2023

nilason commented Sep 15, 2023

cmbarton commented Sep 15, 2023

nilason commented Sep 15, 2023

cmbarton commented Sep 15, 2023 via email

wxGUI/psmap: fix close vector map properties dialog after hit OK button #3085

wxGUI/psmap: fix close vector map properties dialog after hit OK button #3085

Conversation

tmszi commented Jul 14, 2023

cmbarton commented Sep 12, 2023

petrasovaa commented Sep 13, 2023

tmszi commented Sep 13, 2023

cmbarton commented Sep 13, 2023

petrasovaa commented Sep 14, 2023 • edited Loading

tmszi commented Sep 14, 2023 • edited Loading

nilason commented Sep 14, 2023

nilason commented Sep 14, 2023

nilason commented Sep 14, 2023

petrasovaa commented Sep 14, 2023

nilason commented Sep 14, 2023

nilason commented Sep 14, 2023

cmbarton commented Sep 14, 2023

nilason commented Sep 14, 2023

petrasovaa commented Sep 15, 2023

petrasovaa commented Sep 15, 2023

tmszi commented Sep 15, 2023

nilason commented Sep 15, 2023

nilason commented Sep 15, 2023

cmbarton commented Sep 15, 2023

petrasovaa commented Sep 15, 2023

nilason commented Sep 15, 2023

cmbarton commented Sep 15, 2023

nilason commented Sep 15, 2023

cmbarton commented Sep 15, 2023 via email

petrasovaa commented Sep 14, 2023 •

edited

Loading

tmszi commented Sep 14, 2023 •

edited

Loading